-
Notifications
You must be signed in to change notification settings - Fork 23.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[IMP] web: replace wkhtmltopdf with chromeheadless. #32624
[IMP] web: replace wkhtmltopdf with chromeheadless. #32624
Conversation
def _get_wkhtmltopdf_bin(): | ||
return find_in_path('wkhtmltopdf') | ||
def _get_chrome_bin(): | ||
return find_in_path('google-chrome') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not reuse odoo.tests.common.ChromeBrowser
?
At least please do not forget to add support for chromium. 🙏
}) | ||
chrome.navigate_to("file://" + path, True) | ||
res_id = chrome._websocket_send('Page.printToPDF', params=params) | ||
res = chrome._websocket_wait_id(res_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After analyzing the new code, I see that you still don't run the PDF creations asynchronous.
We have done some testing with wkhtml2pdf and lunching the PDF creation asynchronous improves a lot the time to generate a PDF with multiple Odoo records.
If the _websocket_wait_id
was done outside the loop, with the list of processes, the PDF generation would be a lot faster when printing multiple records.
5b62f91
to
99fb080
Compare
99fb080
to
b9988a0
Compare
b9988a0
to
a07b7bf
Compare
a07b7bf
to
adffbd1
Compare
hi @JKE-be. Could you explain the reason of this move ? why leaving wkhtmltopdf ? why choosing google-chrome ? I'm Quite worried to see google technologies landing in all projects. Ref : https://redalemeden.com/blog/2019/we-need-chrome-no-more kind regards. |
adffbd1
to
8a49abe
Compare
@legalsylvain this branch is a test, to see the limitations, nothing has really been decided yet. |
Alternatives:
OTOH:
So I think that this is the less worse solution currently. Possibly using firefox would be cooler, but they don't support it, so... 🤷♂️ |
Hello. (Disclaimer: I'm WeasyPrint's main developer 😉) There is a somehow related thread (OCA/reporting-engine#254), with comments that can be useful about WeasyPrint's pros and cons. Short summary: there are many really useful features for print in WeasyPrint, it's written in Python, but it's pretty slow and doesn't support JS. If the goal is to render the pages the same way they are displayed on screen, with no extra work from the web developer if possible, using a browser engine is a much better idea. WeasyPrint is able to generate high quality documents with the possibility to handle pagination, but it will probably need extra CSS.
These two points are good, but they also are exactly why Chrome/Chromium is becoming a monopoly: we use it more because we already use it, and devs will it more because we assume that they already use it. Is it a good idea to fight for end users' freedom when they love their almighty master? 🤔
I agree, it's the less worse solution on the technical side. PyQt (based on WebKit) is probably a good solution too, and it would avoid depending more on Google products. |
@liZe thank you for your comment. I like your software and would prefer to use it instead of chrome. We could work to remove the JS need (which is probably for wrong reasons) but we do have a need to print really big files sometimes (end of year general ledger can be more than 1000 pages). To update on this task, this is currently blocked as the no external resources in header/footer in headless chrome is quite blocking. |
Oh, that's an interesting article! Here are some random thoughts about these limitations:
I'd be happy to help, even to use other headless browsers 😉. The best I can do is probably to give dirty/smart CSS tricks if needed. |
Is this test going to be continued? |
Not at the moment at least |
Just noting, that maintainer of wkhtmltopdf suggested to use https://github.com/Kozea/WeasyPrint |
If you’re interested in using WeasyPrint, you can ping us, we’ll be happy to help. Since my previous comment, we even have good news. WeasyPrint is now developped by CourtBouillon with official professional support, and backers who financially help us to fix bugs and develop features faster than ever. Headers and footers gained the possibility to include complex layouts thanks to running elements, it should thus be possible to have the features needed for Odoo regarding this topic. We now also support parallel flows in the latest version. |
Description of the issue/feature this PR addresses:
Current behavior before PR:
Desired behavior after PR is merged:
--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr